Skip to content

Conversation

DrSergei
Copy link
Contributor

This patch adds support for the memory event from the DAP. After this event is emitted, VS Code refetches the corresponding memory range and re-renders the memory view. I think this patch and PR can improve experience for users who use setVariable request.

@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2025

@llvm/pr-subscribers-lldb

Author: Druzhkov Sergei (DrSergei)

Changes

This patch adds support for the memory event from the DAP. After this event is emitted, VS Code refetches the corresponding memory range and re-renders the memory view. I think this patch and PR can improve experience for users who use setVariable request.


Full diff: https://github.com/llvm/llvm-project/pull/158437.diff

8 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+4)
  • (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+9)
  • (modified) lldb/tools/lldb-dap/EventHelper.cpp (+13)
  • (modified) lldb/tools/lldb-dap/EventHelper.h (+2)
  • (modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+3)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp (+8)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.h (+28)
  • (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+13)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 9fe8ca22e820b..3a9e076eff9f0 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -216,6 +216,7 @@ def __init__(
         self.events: List[Event] = []
         self.progress_events: List[Event] = []
         self.invalidated_event: Optional[Event] = None
+        self.memory_event: Optional[Event] = None
         self.reverse_requests: List[Request] = []
         self.module_events: List[Dict] = []
         self.sequence: int = 1
@@ -443,6 +444,8 @@ def _handle_event(self, packet: Event) -> None:
             self.capabilities.update(body["capabilities"])
         elif event == "invalidated":
             self.invalidated_event = packet
+        elif event == "memory":
+            self.memory_event = packet
 
     def _handle_reverse_request(self, request: Request) -> None:
         if request in self.reverse_requests:
@@ -1018,6 +1021,7 @@ def request_initialize(self, sourceInitFile=False):
                 "supportsStartDebuggingRequest": True,
                 "supportsProgressReporting": True,
                 "supportsInvalidatedEvent": True,
+                "supportsMemoryEvent": True,
                 "$__lldb_sourceInitFile": sourceInitFile,
             },
         }
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a0a009ae6cc9a..882eec9971a73 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -248,6 +248,14 @@ def verify_invalidated_event(self, expected_areas):
         areas = event["body"].get("areas", [])
         self.assertEqual(set(expected_areas), set(areas))
 
+    def verify_memory_event(self, memoryReference):
+        if memoryReference is None:
+            self.assertIsNone(self.dap_server.memory_event)
+        event = self.dap_server.memory_event
+        self.dap_server.memory_event = None
+        self.assertIsNotNone(event)
+        self.assertEqual(memoryReference, event["body"].get("memoryReference"))
+
     def get_dict_value(self, d: dict, key_path: list[str]) -> Any:
         """Verify each key in the key_path array is in contained in each
         dictionary within "d". Assert if any key isn't in the
@@ -364,6 +372,7 @@ def set_variable(self, varRef, name, value, id=None):
         response = self.dap_server.request_setVariable(varRef, name, str(value), id=id)
         if response["success"]:
             self.verify_invalidated_event(["variables"])
+            self.verify_memory_event(response["body"].get("memoryReference"))
         return response
 
     def set_local(self, name, value, id=None):
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 6eb468e76b16c..9c2c1f846d11e 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -284,4 +284,17 @@ void SendInvalidatedEvent(
   dap.Send(protocol::Event{"invalidated", std::move(body)});
 }
 
+void SendMemoryEvent(DAP &dap, lldb::SBValue variable) {
+  if (!dap.clientFeatures.contains(protocol::eClientFeatureMemoryEvent))
+    return;
+  const lldb::addr_t addr = variable.GetLoadAddress();
+  if (addr == LLDB_INVALID_ADDRESS)
+    return;
+  protocol::MemoryEventBody body;
+  body.memoryReference = addr;
+  body.offset = 0;
+  body.count = variable.GetByteSize();
+  dap.Send(protocol::Event{"memory", std::move(body)});
+}
+
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 0c57afbaf1f33..48eb5af6bd0b9 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -37,6 +37,8 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
 void SendInvalidatedEvent(
     DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas);
 
+void SendMemoryEvent(DAP &dap, lldb::SBValue variable);
+
 } // namespace lldb_dap
 
 #endif
diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
index 2a50dea0b4ada..b9ae28d6772ac 100644
--- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
@@ -82,6 +82,9 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
   // (e.g. references) can be changed.
   SendInvalidatedEvent(dap, {InvalidatedEventBody::eAreaVariables});
 
+  // Also send memory event to signal client that variable memory was changed.
+  SendMemoryEvent(dap, variable);
+
   return body;
 }
 
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
index 9598c69878d66..98bc18f880aa8 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Protocol/ProtocolEvents.h"
+#include "JSONUtils.h"
 #include "llvm/Support/JSON.h"
 
 using namespace llvm;
@@ -56,4 +57,11 @@ llvm::json::Value toJSON(const InvalidatedEventBody &IEB) {
   return Result;
 }
 
+llvm::json::Value toJSON(const MemoryEventBody &MEB) {
+  return json::Object{
+      {"memoryReference", EncodeMemoryReference(MEB.memoryReference)},
+      {"offset", MEB.offset},
+      {"count", MEB.count}};
+}
+
 } // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
index 138b622e01210..045b2ac7d8ea8 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
@@ -88,6 +88,34 @@ struct InvalidatedEventBody {
 llvm::json::Value toJSON(const InvalidatedEventBody::Area &);
 llvm::json::Value toJSON(const InvalidatedEventBody &);
 
+/// This event indicates that some memory range has been updated. It should only
+/// be sent if the corresponding capability supportsMemoryEvent is true.
+///
+/// Clients typically react to the event by re-issuing a readMemory request if
+/// they show the memory identified by the memoryReference and if the updated
+/// memory range overlaps the displayed range. Clients should not make
+/// assumptions how individual memory references relate to each other, so they
+/// should not assume that they are part of a single continuous address range
+/// and might overlap.
+///
+/// Debug adapters can use this event to indicate that the contents of a memory
+/// range has changed due to some other request like setVariable or
+/// setExpression. Debug adapters are not expected to emit this event for each
+/// and every memory change of a running program, because that information is
+/// typically not available from debuggers and it would flood clients with too
+/// many events.
+struct MemoryEventBody {
+  /// Memory reference of a memory range that has been updated.
+  lldb::addr_t memoryReference;
+
+  /// Starting offset in bytes where memory has been updated. Can be negative.
+  int64_t offset;
+
+  /// Number of bytes updated.
+  uint64_t count;
+};
+llvm::json::Value toJSON(const MemoryEventBody &);
+
 } // end namespace lldb_dap::protocol
 
 #endif
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp
index a964592495347..be1e6a1470252 100644
--- a/lldb/unittests/DAP/ProtocolTypesTest.cpp
+++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp
@@ -1088,3 +1088,16 @@ TEST(ProtocolTypesTest, InvalidatedEventBody) {
 })";
   EXPECT_EQ(json, pp(body));
 }
+
+TEST(ProtocolTypesTest, MemoryEventBody) {
+  MemoryEventBody body;
+  body.memoryReference = 12345;
+  body.offset = 0;
+  body.count = 4;
+  StringRef json = R"({
+  "count": 4,
+  "memoryReference": "0x3039",
+  "offset": 0
+})";
+  EXPECT_EQ(json, pp(body));
+}

@JDevlieghere JDevlieghere requested a review from ashgti September 15, 2025 17:16
Copy link
Contributor

@ashgti ashgti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines 108 to 115
/// Memory reference of a memory range that has been updated.
lldb::addr_t memoryReference;

/// Starting offset in bytes where memory has been updated. Can be negative.
int64_t offset;

/// Number of bytes updated.
uint64_t count;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a default of 0 or a some define/const value for these?

In case we don't need to set a value.

@ashgti
Copy link
Contributor

ashgti commented Sep 15, 2025

memoryReference can default to LLDB_INVALID_ADDRESS and count should probably default to 0 as well.

@DrSergei
Copy link
Contributor Author

memoryReference can default to LLDB_INVALID_ADDRESS and count should probably default to 0 as well.

I think it is not the best solution. These fields are required, so we should initialize them with meaningful values. UBSan can potentially find situation where initialization was missed if default value not provided. I can add these default values if you disagree with my opinion.

@ashgti
Copy link
Contributor

ashgti commented Sep 15, 2025

Declaring a variable, should just work in case we don't set a value.

If we wanted, we could assert in toJSON, e.g. assert that the assert(memoryReference != LLDB_INVALID_ADDRESS); to catch incorrectly constructed values.

But doing MemoryEventBody body; should ideally be safe. Right now, I think it would assign a random~ish value to memoryReference and count.

@JDevlieghere
Copy link
Member

@ashgti Would you like to take another look at this or can I merge this on @DrSergei's behalf?

@ashgti
Copy link
Contributor

ashgti commented Sep 17, 2025

LGTM, I can merge it.

@ashgti ashgti merged commit c744f61 into llvm:main Sep 17, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants